Skip to content

fix: read Lighter order and trade ids from the string fields (ORD-2039) - #616

Merged
aaronmboyd merged 2 commits into
mainfrom
feature/ord-2039
Oct 9, 2026
Merged

fix: read Lighter order and trade ids from the string fields (ORD-2039)#616
aaronmboyd merged 2 commits into
mainfrom
feature/ord-2039

Conversation

@aaronmboyd

@aaronmboyd aaronmboyd commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Closes

Linear: ORD-2039

Intent

Lighter order indexes are above 2^53 on every market with a market id of 31 or more. JSON.parse rounds these numbers. For example, 9570149379440277 becomes 9570149379440276. The SDK read the numeric fields, so Order.orderId, Order.clientOrderId, Fill.id and Fill.orderId were wrong, and getOrder did not find the order. This PR reads the exact string fields that Lighter sends.

What changed

  • types/fill.ts: LtTrade declares trade_id_str, ask_id_str, bid_id_str, ask_client_id_str and bid_client_id_str as string.
  • utils/mapFill.ts: Fill.id comes from trade_id_str. Fill.orderId comes from bid_id_str or ask_id_str.
  • utils/mapOrder.ts: orderId comes from order_id. clientOrderId comes from client_order_id, and the mapper omits it when the value is "0". mapOrderUpdates uses order_id in the terminated list and in the dropped-row text.
  • LighterProvider.ts: the getOrder lookup compares order_id. The client order lookup compares client_order_id.
  • The WebSocket fill and order paths call the same mappers, so they get the same fix.
  • .changeset/ord-2039.md: patch for @lifi/perps-sdk-provider-lighter.

Assumptions

  • The "no client order id" value is the string "0". The mainnet recentTrades response sends ask_client_id: 0 with ask_client_id_str: "0", and the OpenAPI spec declares these fields as required strings. A live authenticated order read was not done; a human run can verify it.
  • ask_client_id_str and bid_client_id_str are declared because the API sends them. No code reads them at this time.
  • The outbound signer, wasm/, the numeric order_index in src/types/action.ts and LtOrderBookOrder do not change.

Tests

  • mapFill.unit.spec.ts, mapOrder.unit.spec.ts and LighterProvider.unit.spec.ts use the fixture id 9570149379440277 and assert the exact string in the output. The new LighterProvider lookup tests fail on the old code.
  • LighterWsProvider.unit.spec.ts sends the literal unsafe number on the wire, so JSON.parse rounds it. The test asserts that the WebSocket output is equal to the mapFill and mapOrder output.
  • pnpm test:unit passes in all packages (lighter 1243). pnpm check, check:types, check:types:spec and build pass.

Follow-ups

  • ORD-2040 moves the Lighter signer to web-wasm, which takes the order index as a string. That issue exists already.

Dependencies

None.

@aaronmboyd aaronmboyd added release-preview Publish a 0.0.0-preview-<sha> build of this PR to npm Agent Review Request labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

📦 Preview published under the preview dist-tag.

Install the exact version(s) — @preview moves with the newest preview across PRs:

npm i @lifi/perps-sdk-provider-hyperliquid@0.0.0-preview-f336ff4
npm i @lifi/perps-sdk-provider-lighter@0.0.0-preview-f336ff4
npm i @lifi/perps-sdk@0.0.0-preview-f336ff4

@github-actions github-actions Bot removed the release-preview Publish a 0.0.0-preview-<sha> build of this PR to npm label Oct 9, 2026
@lifi-qa-agent

lifi-qa-agent Bot commented Oct 9, 2026

Copy link
Copy Markdown

QA Review — #616 (ORD-2039)

Verdict: ✅ Approve · Critical 0 · High 0 · Medium 0 · Low 0 · Awaiting response 0 · Review type: 🆕 First review (focused direct pass, head f336ff45)

Summary

Reads Lighter order, client order and trade ids from the exact string fields (order_id, client_order_id, trade_id_str, bid_id_str, ask_id_str), so ids above 2^53 no longer round in Order.orderId, Order.clientOrderId, Fill.id, Fill.orderId or the getOrder lookups.

Findings

None.

Verified against the clone at head

  • Every read site named in ORD-2039 is converted. Remaining order_index / client_order_index references are the outbound signer types and a request parameter, which the ticket puts out of scope.
  • LtOrder already declares order_id and client_order_id as strings; LtTrade now declares the five *_str twins.
  • Both mapFill callers (REST getFills, WS account trades) go through the same mapper, so the WebSocket path gets the fix.
  • clientOrderId is omitted when the value is "0", matching the author's cited mainnet payload. The getOrder client-index lookup compares strings, and the id is already pattern-checked as a decimal integer first.
  • Specs use fixture id 9570149379440277 and assert the exact string. The WS spec sends the unsafe number on the wire and compares against the mapper output.
  • Patch changeset for @lifi/perps-sdk-provider-lighter is correct.

🔵 Notes (not findings)

  • CI was not verified and the local gates were not re-run; the pass counts are author-reported.
  • The live authenticated check that an order without a client index returns client_order_id: "0" is deferred to a human, as the author disclosed.
  • Making the five *_str members required on the public LtTrade type could break a consumer that builds fixtures, but it matches the Lighter API spec and the ticket.

lifi-qa-agent[bot]
lifi-qa-agent Bot approved these changes Oct 9, 2026
View reviewed changes

@lifi-qa-agent lifi-qa-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review: approved, no findings. Full report in the QA comment above.

@aaronmboyd
aaronmboyd merged commit 662a413 into main Oct 9, 2026
2 checks passed
@aaronmboyd
aaronmboyd deleted the feature/ord-2039 branch October 9, 2026 14:15
@github-actions github-actions Bot mentioned this pull request Oct 9, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lifi-qa-agent lifi-qa-agent[bot] lifi-qa-agent[bot] approved these changes

Assignees

No one assigned

Labels

QA AI Approved

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@aaronmboyd